Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run CI tests in parallel #25271

Merged
merged 6 commits into from
Jan 9, 2025
Merged

Run CI tests in parallel #25271

merged 6 commits into from
Jan 9, 2025

Conversation

dantecatalfamo
Copy link
Member

@dantecatalfamo dantecatalfamo commented Jan 8, 2025

#21774

Improves run time by about 30%.

Things have been arranged in such a way that splitting modules out further will be trivial in the future, such as breaking the different integration test suited into their own units.

image

image

@dantecatalfamo dantecatalfamo changed the title Try splitting tests up Split up go tests Jan 8, 2025
@dantecatalfamo dantecatalfamo changed the title Split up go tests Split up go tests in CI Jan 8, 2025
Copy link

codecov bot commented Jan 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.83%. Comparing base (8d09035) to head (b7c4f34).
Report is 34 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25271      +/-   ##
==========================================
- Coverage   63.85%   63.83%   -0.03%     
==========================================
  Files        1616     1616              
  Lines      153836   153851      +15     
  Branches     3976     3976              
==========================================
- Hits        98230    98207      -23     
- Misses      47791    47838      +47     
+ Partials     7815     7806       -9     
Flag Coverage Δ
backend 64.70% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dantecatalfamo dantecatalfamo marked this pull request as ready for review January 8, 2025 22:04
@dantecatalfamo dantecatalfamo changed the title Split up go tests in CI Run CI tests in parallel Jan 8, 2025
Makefile Outdated Show resolved Hide resolved
lucasmrod
lucasmrod previously approved these changes Jan 8, 2025
Copy link
Member

@lucasmrod lucasmrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Left a nit comment around ./ prefix in the paths but if this works, then g2g.

@dantecatalfamo
Copy link
Member Author

@lucasmrod I'm running a new test right now with all of server/datastore/mysql as its own test outside of core, I'm going to see if it's any faster than only running server/datastore/mysql/migrations on its own

@dantecatalfamo
Copy link
Member Author

It's about the same, the longest test is still the integration test. In the future we could run the Enterprise and MDM integration tests as their own unit to speed it up

PKG_TO_TEST := "" # default to empty string; can be overridden on command line.
go_test_pkg_to_test := $(addprefix ./,$(PKG_TO_TEST)) # set paths for packages to test
dlv_test_pkg_to_test := $(addprefix github.com/fleetdm/fleet/v4/,$(PKG_TO_TEST)) # set URIs for packages to debug

DEFAULT_PKG_TO_TEST := ./cmd/... ./ee/... ./orbit/pkg/... ./orbit/cmd/orbit ./pkg/... ./server/... ./tools/...
ifeq ($(CI_TEST_PKG), main)
CI_PKG_TO_TEST=$(shell go list ${DEFAULT_PKG_TO_TEST} | grep -v "server/datastore/mysql" | grep -v "cmd/fleetctl" | grep -v "server/vulnerabilities" | sed -e 's|github.com/fleetdm/fleet/v4/||g')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a limit to the # of packages you can specify for go test, so as long as it fits in one shell command we should be 👍

Makefile Outdated Show resolved Hide resolved
@dantecatalfamo
Copy link
Member Author

I think if we split up the different integration tests into their own units we could save time as well, but I also don't know how far down that rabbit hole I want to go due to potential github action costs.

If we could somehow do the schema dump and mock generation in their own action and then reuse it in the other actions, we could probably save ~1.5m per test. Maybe something to look into in another ticket.

Copy link
Collaborator

@sharon-fdm sharon-fdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dantecatalfamo dantecatalfamo merged commit e6fb647 into main Jan 9, 2025
15 checks passed
@dantecatalfamo dantecatalfamo deleted the 21774-split-go-tests branch January 9, 2025 18:38
dantecatalfamo added a commit that referenced this pull request Jan 10, 2025
Follow up to #25271 and #21774

Integration test failures will happen much faster of they occur, but now
the bottleneck is the `fleetctl` test suite.

It's trivial to continue splitting tests up now. We should look into
creating an action that checks that mock generation is up-to-date, run
it before all the tests, and then remove the mock generation step from
each test step. That would save about a minute and a half of runtime
from each test and help offset the cost of splitting the tests up.

![ci runtime
breakdown](https://github.com/user-attachments/assets/057b8ee1-782c-4e1f-9486-42c7d1169c81)
![ci runtime
max](https://github.com/user-attachments/assets/3a26995f-d9cb-490b-84d9-1a7fbb3cd6b3)

![image](https://github.com/user-attachments/assets/b4c888c8-867f-4bdd-9b69-0dc20d0d202a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants